-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Simplify TypeComparer #9405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify TypeComparer #9405
Conversation
ec87f60
to
e190c06
Compare
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9405/ to see the changes. Benchmarks is based on merging with master (239f4e3) |
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9405/ to see the changes. Benchmarks is based on merging with master (d6d374a) |
Have a separate stack of type comparers instead.
d83d672
to
5e0dc9c
Compare
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9405/ to see the changes. Benchmarks is based on merging with master (15ae0aa) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
def reset(): Unit = { | ||
for ((_, set) <- uniqueSets) set.clear() | ||
errorTypeMsg.clear() | ||
sources.clear() | ||
sourceNamed.clear() | ||
comparers.clear() // forces re-evaluation of top and bottom classes in TypeComparer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to clear exploreContexts
, and reset exploresInUse
and comparersInUse
to 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, every nested call with reset to the previous state after itself, even under exceptions. So there's nothing to do here.
We create by accident a TypeComparer for every context. In my typer/*.scala benchmark, that created a million TypeComparers. Each of them has about 20 fields. We now create 3 TypeComparers overall for the same benchmark. This is done by having a stack of TypeComparers in use and re-using TypeComparers that were popped from the stack before.